Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enable attendee selection on shared calendars #6202

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Jul 24, 2024

enable attendee selection on shared calendars

This is needed to test shared calendar event invitation sending nextcloud/server#45054

@SebastianKrupinski SebastianKrupinski self-assigned this Jul 24, 2024
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 23.70%. Comparing base (8dbff4c) to head (34de776).
Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6202      +/-   ##
============================================
+ Coverage     23.67%   23.70%   +0.02%     
  Complexity      457      457              
============================================
  Files           249      249              
  Lines         11774    11763      -11     
  Branches       2203     2210       +7     
============================================
  Hits           2788     2788              
+ Misses         8669     8659      -10     
+ Partials        317      316       -1     
Flag Coverage Δ
javascript 15.27% <ø> (+0.01%) ⬆️
php 59.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVM, wrong ics

@miaulalala miaulalala self-requested a review August 1, 2024 10:53
@tcitworld
Copy link
Member

I don't know what's the plan for nextcloud/server#45054 (backports ?), but in any case, this should depend on the server version to be activated.

Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Popover modal is not in sync with sidebar modal:

Popover:
image

Popover Editor:

image

EditorSidebar:

image

@SebastianKrupinski SebastianKrupinski marked this pull request as ready for review August 5, 2024 15:41
@SebastianKrupinski SebastianKrupinski force-pushed the fix/shared-calendar-enable-attendees branch from c66e356 to 34de776 Compare August 6, 2024 12:40
@SebastianKrupinski
Copy link
Contributor Author

@miaulalala the popup modal is NOT out of sync, it just does not have enough room to show all the attendees, it has the same behavior with a personal calendar. This has nothing to do with this PR and would need to be addresses in another PR.

@miaulalala
Copy link
Contributor

@miaulalala the popup modal is NOT out of sync, it just does not have enough room to show all the attendees, it has the same behavior with a personal calendar. This has nothing to do with this PR and would need to be addresses in another PR.

fair enough, can you please open a ticket with these findings?

@miaulalala miaulalala self-requested a review August 6, 2024 12:53
@ChristophWurst
Copy link
Member

I don't know what's the plan for nextcloud/server#45054 (backports ?), but in any case, this should depend on the server version to be activated.

Calendar main is 30+ and we won't backport this to my knowledge. This should be alright :)

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An organizer selection is still missing for the assistant use case, right? As in, Alice shares with her assistant Bob, who invites Charly. Alice is saved as organizer, bob is an attendee.

@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Aug 6, 2024

Additional thoughts...

Some front end UI changes that would be beneficial...

  • Drop down on attendee tab to selected the organizer of the event. Use cases...
  • Sharee is an assistant with access to the Managers calendar (Sharer)... The organizer of the event should be the Sharer.
  • Sharee is a employee with access to the Company calendar (Sharer)... The organizer of the event should be the Sharee.

Some back end changes that would be beneficial...

  • Additional calendar property to control access to send on behalf of the Sharer.

Draw backs...

  • The above features would only work in the NC UI. CalDAV dose support the "CALDAV:schedule-send" ACL permission but I doubt that there are many if any clients that actually support this feature.

@SebastianKrupinski
Copy link
Contributor Author

An organizer selection is still missing for the assistant use case, right? As in, Alice shares with her assistant Bob, who invites Charly. Alice is saved as organizer, bob is an attendee.

Correct. See comments above.

@SebastianKrupinski SebastianKrupinski merged commit 57a5653 into main Aug 6, 2024
28 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/shared-calendar-enable-attendees branch August 6, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature request Feature: Sharing
Projects
Status: ☑️ Done
Development

Successfully merging this pull request may close these issues.

4 participants